PHPC-2230: Drop support for PHP 7.2 and 7.3 #1450
Conversation
There was a problem hiding this comment.
Noted that you're leaving the Serializable implementations in place for now (per PHPC-2248).
I recall there being some EXPECTF patterns that varied by PHP version and decided to search for %r in the tests. That likely doesn't catch everything, but it did turn up quite a bit:
%r(argument|parameter)%rand%r(E_NOTICE|E_WARNING)%rdate back to 60febd0 and might still be relevant.%r(object|stdClass)%ris pretty recent (6887226) and is probably still relevant.%r[eE]%rwas recently introduced in the Int64 tests. I'm not sure if it's was needed for a particular PHP version or platform (e.g. Windows).%r(null|NULL)%rdates back to PHP 5 and can be removed (see: 6512570)int%Sandbool%Saddresses a difference between PHP 7.2 and 7.3 (98e583b). There may be additional instances of%r(int|integer)%ror%r(bool|boolean)%r.%r(double|float)%addresses a difference between PHP 5 and 7 (see: PHPC-790: Accept integer and floats in Timestamp and UTCDateTime constructors #467 (comment)).%r(0|"0")%rwas related to PHP 7.2 (see: 95c4678)%winset_statetests dates back to HHVM (see: c2fe3b2)
This PR would probably be a good time to address any obsolete patterns in the above list.
|
I've cleaned up the following:
I've left the others in place for the time being and will check those separately and add comments where possible to indicate when they can be removed. |
alcaeus
force-pushed
the
phpc-2230-drop-old-php
branch
from
f269ebd to
d33bc7f
Compare
| %r\)?%r), | ||
| 2 =>%w | ||
| %r\(object\)? %rarray( | ||
| %r\)?%r), |
There was a problem hiding this comment.
Why would we expect another optional ) character here? I understand an optional (object) cast in the output, but there is already an expected ) to balance array( on the previous line. And there are other closing parens below that balance out the array( instances up top.
I don't think this has anything to do with your previous related comments:
- PHPC-2230: Drop support for PHP 7.2 and 7.3 #1450 (comment)
- PHPC-2230: Drop support for PHP 7.2 and 7.3 #1450 (comment)
I see you have %r\(object\)? %r above, and the closing paren there is optional. Does that mean that PHP sometimes emits (object array(...))? That doesn't make sense as it wouldn't be valid syntax.
In fact, the %r\(object\)? %r pattern would only match (object or (object) . In that case, it seems like (object) is always present (it's the only valid syntax). So perhaps we should expect (object) for PHP 7.4+.
I tested this in https://3v4l.org/MaXQ3 and (object) is always included in PHP 7.3+. Previous versions would emit stdClass::__set_state(array(...)), which explains the extra closing paren, which is no longer applicable.
And PHP 8.2+ differs slightly by emitting a backslash before class names to make them fully-qualified. This is already accounted for with the %r\\?%r pattern earlier in the EXPECTF block.
Note: this applies to other EXPECTF sections with __set_state() output.
There was a problem hiding this comment.
Good observation, I believe I originally added this in error and then it spread via the usual way of copy/paste.
alcaeus
force-pushed
the
phpc-2230-drop-old-php
branch
2 times, most recently
from
bab167c to
5c195e6
Compare
alcaeus
force-pushed
the
phpc-2230-drop-old-php
branch
from
5c195e6 to
e37a313
Compare
alcaeus
force-pushed
the
phpc-2230-drop-old-php
branch
from
e37a313 to
45ed3d6
Compare
PHPC-2230
This drops support for PHP 7.2 and 7.3, along with a bunch of compatibility code.
I've also deleted all tests regarding serialisation on PHP < 7.4 and updated the
SKIPIFblocks on those tests that affect PHP 7.4+.